Invalidate per-project restore skip when paket.lock content changes#4334
Open
randrag wants to merge 2 commits intofsprojects:masterfrom
Open
Invalidate per-project restore skip when paket.lock content changes#4334randrag wants to merge 2 commits intofsprojects:masterfrom
randrag wants to merge 2 commits intofsprojects:masterfrom
Conversation
…ket.props stale Step 2a's per-project skip optimisation in Paket.Restore.targets only invalidates on paket.references content change. When a merge bumps a transitive package version, paket.lock and paket.resolved change but paket.references does not, and (with BaseIntermediateOutputPath redirected per-platform) the per-project paket.props stays at the pre-merge content. NuGet then resolves to the previous version indefinitely. This test reproduces that path: full restore, mutate paket.lock content, expect the next restore to invalidate the per-project skip. With the unfixed targets file the skip stays in effect and the test fails. Companion fix to follow.
Add Step 2 a-bis to Paket.Restore.targets that hashes paket.lock and compares to a per-project hash cache file written after each successful per-project restore. If the cached hash differs from the current hash, the per-project skip is invalidated and a paket restore runs. A fall-through case fires once on first encounter for legacy checkouts that pre-date the cache file, so installations upgrading to this version self-correct on the next restore. This complements Step 2a (which hashes paket.references content) so that pure-transitive package version changes — where paket.lock and paket.resolved change but paket.references does not — also invalidate the per-project skip. The hash is computed locally rather than reusing $(PaketRestoreLockFileHash) from Step 1 because Step 1 may be disabled via PaketDisableGlobalRestore. A new file $(PaketIntermediateOutputPath)\<proj>.fsproj.paket.lock.hash records the lock hash at the time of the last successful per-project restore. Greens the integration test added in the previous commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is to fix #4333 — Step 2a's skip in
Paket.Restore.targetspreviously only watchedpaket.references, so transitive changes inpaket.lockleft the per-projectpaket.propsstale. The fix adds apaket.lockcontent-hash check (Step 2 a-bis) and a one-line write of that hash after each successful per-project restore (Step 3 a-bis), plus a fall-through that fires once on first encounter for legacy checkouts that pre-date the cache file.Test in
RestoreSpecs.fsfollows the existing#2684/#3527patterns: full restore, mutatepaket.lockcontent, assert that the nextdotnet restorerequests a per-project restore (viaPAKET_ERROR_ON_MSBUILD_EXEC=true).